Skip to content

fix: reject malformed varints in compressed batches instead of panicking or misreading - #908

Open
solace-aross wants to merge 2 commits into
mainfrom
fix/sol-155081-snappy-fuzz-target
Open

solace-aross wants to merge 2 commits into
mainfrom
fix/sol-155081-snappy-fuzz-target

Conversation

@solace-aross

@solace-aross solace-aross commented Oct 9, 2026 •

Copy link
Copy Markdown
Collaborator

What changes, and why?

A record in a compressed batch whose varint field has more continuation bytes than the field can hold is now rejected as corrupt. Before, it panicked in builds with overflow checks and decoded to a silently wrapped value in release builds ([profile.release] sets no overflow-checks).

  • Fix: overlong varint. The serde VarInt and LongVarInt deserializers, used to decode records from a compressed batch, shifted and added without overflow checks. They now use checked arithmetic and return an error, as UnsignedVarInt and both Decode implementations already do. Like those, this rejects a shift past the type's width; it does not reject high bits lost on the final byte.
  • fuzz_batch_records (new), which found the bug. The existing fuzz_deflated_batch target only parsed the batch header, so the decompressors and the record decoder were never fuzzed. The first input byte selects the codec, the next two give the record count, and the rest is the record data. It runs Vec::<Record>::try_from and inflated::Batch::try_from, by reference and by value, as the storage engines do, and checks that the two Vec<Record> results agree.
    • One difference is allowed on purpose. For an uncompressed batch, the by-value conversion rejects bytes left after the last record and the by-reference conversion accepts them. Without the exception the fuzzer reports it within seconds and finds nothing else. The code comment says to remove it once the by-reference conversion rejects trailing bytes. Compressed batches share one implementation, so they must agree.
  • fuzz_deflated_batch now also decodes the records of a batch it parses.
  • generate_seeds writes a valid seed per codec, a xerial-framed Snappy seed, and the xerial magic followed by 0 to 11 bytes.
  • justfile: fix fuzz-generate-seed (cargo fuzz run does not accept --package or --bin) and add fuzz-batch-records.

The Snappy xerial header slice panic that motivated this work is already fixed on main (#821). This change does not touch that code.

Upgrade impact

None. A batch that previously decoded to a wrapped value, or panicked, now fails to decode as corrupt.

How was this tested?

  • The two new varint tests fail on main (attempt to shift left with overflow at varint.rs:159 and :354) and pass with the fix.
  • just fuzz-generate-seed, then just cargo-fuzz run fuzz_batch_records -- -max_total_time=600: 923,961 executions, no crash.
  • After rebasing on main: just fmt, just clippy (workspace, all targets, -D warnings, which covers the fuzz crate) and cargo nextest run -p nisshi-sans-io --all-features (400 passed) pass locally. CI runs the other crates' tests.
  • No CI job runs a fuzz target.

🤖 Generated with Claude Code

@solace-aross
solace-aross force-pushed the fix/sol-155081-snappy-fuzz-target branch from b74d996 to a372edc Compare October 9, 2026 17:04
@solace-aross solace-aross changed the title fuzz: decode batch records for every codec; reject an overlong varint in record data fix: reject malformed varints in compressed batches instead of panicking or misreading Oct 9, 2026
solace-aross and others added 2 commits October 9, 2026 16:48
The existing fuzz_deflated_batch target only parsed the batch header, so
the decompressors and the record decoder were never fuzzed. A Snappy
batch whose record data was the xerial magic followed by fewer than 12
bytes panicked in Compression::inflator and went unnoticed (SOL-155081).

- Add fuzz_batch_records: the first input byte selects the codec, the
  next two give the record count, the rest is the record data. It runs
  Vec::<Record>::try_from and inflated::Batch::try_from, by reference
  and by value, and asserts the two Vec::<Record> conversions agree.
- fuzz_deflated_batch now also decodes the records of a parsed batch.
- generate_seeds writes a valid seed per codec, a xerial-framed Snappy
  seed, and the magic plus 0 to 11 bytes.
- Fix the fuzz-generate-seed recipe (cargo fuzz run does not accept
  --package or --bin) and add a fuzz-batch-records recipe.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Andrea Ross <168456375+solace-aross@users.noreply.github.com>
The serde VarInt and LongVarInt deserializers, used to decode records
from a compressed batch, shifted and added without overflow checks. A
varint with more continuation bytes than its type can hold panicked with
"attempt to shift left with overflow" in builds with overflow checks,
and decoded to a silently wrapped value in builds without them.

Use checked arithmetic and return an error, as UnsignedVarInt and both
Decode implementations already do. Found by the fuzz_batch_records
target (SOL-155081).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Andrea Ross <168456375+solace-aross@users.noreply.github.com>
@solace-aross
solace-aross force-pushed the fix/sol-155081-snappy-fuzz-target branch from a372edc to 94e0abc Compare October 9, 2026 20:54

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant